Repository navigation
conformance(report): include TR-APR in LEVEL_MODULES[0] and guard non-dict trace in _assemble - #564
Conversation
rajnisht7
left a comment
There was a problem hiding this comment.
LEVEL_MODULES parity and the non-dict trace guard looks correct.
however the issue is record_digest, it now goes through canonical_claim_bytes, so floats / NaN / unsafe integers raise InclusionError. _assemble catches that and falls back to a different json.dumps(..., allow_nan=True) digest so build() still works, good for reportability, but it leaves two digest regimes for the same record:
record_digest(record) -> raises
report.build(...).digest -> fallback hash
this report still tells readers to “compare the digest you compute,” but for boundary records they cant get that value from record_digest() and the docstring also don’t document the new strict contract or the fallback
so either:
- keep record_digest as a total diagnostic identifier and use canonical_claim_bytes only where anchor admissibility is required, or
- Document strict record_digest (InclusionError out of profile), treat the _assemble fallback as diagnostic-only, and add e2e tests that build / HTML / JSON succeed with a deterministic fallback digest for NaN, 1.5, and 2**60, and that this isn’t presented as a registry-anchor digest.
also worth noting:
- The fallback uses allow_nan=True; if you keep it, state clearly that it’s diagnostic-only and not registry-anchor canonicalization.
- A separate digest_profile (or similar) would make the two regimes explicit for readers.
- The non-dict trace test only catches AttributeError; asserting the report fields is enough for now, but a broader “must not raise” would be slightly stronger.
- Existing obligation-accounting boundary tests stay green via the fallback; the gap is the public record_digest contract and e2e coverage of that fallback path.
|
Thanks @drk-ram, and thanks @rajnisht7 for laying out the two options. Let's take option 1 and keep The report is there to describe records that fail, including ones with floats, So please drop the switch to |
Companion to drk-ram PR agentrust-io#564, preserving original ownership. Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@users.noreply.github.com>
dinakarjs
left a comment
There was a problem hiding this comment.
Exact-head check of c10ff01: Imran's diagnostic-digest adjustment is not yet present. Companion for @drk-ram to cherry-pick into this original PR, retaining ownership: dinakarjs@783f06d . It preserves TR-APR parity and the non-dict trace guard, restores the original diagnostic serialization, removes the second fallback regime, and tests public digest/build/HTML/JSON agreement for NaN, infinities, fractions and unsafe integers.
Validation on the companion changes, with exact checkout imports: full conformance 791 passed, 5 xpassed after building/installing this checkout's wheel into an isolated metadata target; 12 root parity tests passed; ruff passed. Independent cold read found no blocking issue; its clearer-test-name suggestion is incorporated. Creating the companion PR directly into the author's fork returned GitHub 403, so handing off the DCO-signed commit here. This is not approval of the unchanged current head; no merge requested.
Companion to drk-ram PR agentrust-io#564, preserving original ownership. Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com> Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@users.noreply.github.com> Co-authored-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Thank you @imran-siddique, @rajnisht7, and @dinakarjs! I have cherry-picked @dinakarjs's companion commit (
|
dinakarjs
left a comment
There was a problem hiding this comment.
Adoption verified at exact head 6465366. Imran's selected single diagnostic-digest contract is implemented. TR-APR parity and the non-object trace guard remain, while digest/build/JSON/HTML agree for NaN, infinities, fractions and unsafe integers without a second fallback regime.
With this head's hash-locked root/conformance dependencies and explicit checkout import paths, all 791 conformance tests pass with five xpasses; the 12 root parity controls also pass. These suites run separately because their tests packages collide when collected together. Restoring strict canonical-claim hashing only in memory makes six diagnostic-boundary controls fail while the other six controls pass. Ruff, targeted strict mypy for report.py and the root parity module, root-library mypy, dash and whitespace checks pass. CI and Conformance CI succeed; ClusterFuzzLite was still running when checked.
No additional blocking finding in this adoption check. I authored the companion digest correction, so this is incorporation/regression evidence rather than an independent maintainer approval of my own changes. Diagnostic identity remains distinct from registry-anchor admissibility; five xpasses are not five additional acceptance requirements.
rajnisht7
left a comment
There was a problem hiding this comment.
changes looks good, LEVEL_MODULES[0] now matches the runner, non-dict trace is guarded cleanly, and record_digest stays a single diagnostic path as ruled. Tests cover the cases.
…igest - Include "TR-APR" in report.LEVEL_MODULES[0] to match runner._LEVEL_MODULES[0]. - Guard non-dict record["trace"] values in report._assemble so malformed CMCP records report transparency=None instead of raising AttributeError. - Delegate report.record_digest to inclusion.canonical_claim_bytes so report digests enforce the registry-anchor-v1 section 1 profile, with a fallback in report._assemble when building reports over negative-test boundary records. Closes agentrust-io#557 Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Companion to drk-ram PR agentrust-io#564, preserving original ownership. Signed-off-by: Raghuvar Durga <177662415+drk-ram@users.noreply.github.com> Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@users.noreply.github.com> Co-authored-by: Srinivasa Consultants <78673153+dinakarjs@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
6465366 to
7d16c5b
Compare
imran-siddique
left a comment
There was a problem hiding this comment.
Thanks @drk-ram and @dinakarjs. This is option 1 from 17 October: record_digest stays one digest for every record, with no fallback hash, and the TR-APR and non-object trace fixes are kept. @rajnisht7 your approval was dismissed only by the rebase; the two test files are unchanged and report.py differs from your approved head only by what main brought in.
What this changes
Contributor: Raghuvar Durga (@drk-ram)
Closes #557
Fixes two defects in
conformance/src/trace_tests/report.pyand preservesrecord_digestas a total diagnostic identifier across all records per maintainer review:"TR-APR"inreport.LEVEL_MODULES[0]: UpdatesLEVEL_MODULES[0]from("TR-ENV", "TR-SIG", "TR-POL")to("TR-ENV", "TR-SIG", "TR-POL", "TR-APR")so the report module breakdown matchesrunner._LEVEL_MODULES[0]and cumulative level schedules across Levels 0, 1, and 2.record["trace"]in_assemble: Checksisinstance(raw_trace, dict)before readingtrace.get("transparency")so malformed CMCP records whererecord["trace"]is a non-dict ("not-a-dict",123,None,[1, 2],True) reporttransparency=None(unanchoredin HTML) instead of crashingreport.buildwithAttributeError.record_digestacross all report surfaces: Per @imran-siddique's ruling following @rajnisht7's review (and companion commit783f06dby @dinakarjs),record_digestcontinues to hash sorted-key ASCII JSON for every record (including failing records with floats,NaN,Infinity, or out-of-range integers) without a separate fallback regime, ensuringrecord_digest(record),report.build(...).digest,to_json, andto_htmlalways produce the same digest.Self-review notes:
canonical_claim_bytesfromrecord_digestand remove the second fallback hash in_assemble, sorecord_digestremains a total diagnostic identifier for all records (including failing boundary records) and agrees acrossrecord_digest(),build(),to_json(), andto_html().conformance/src/trace_tests/report.pywith--sopython tools/check_dashes.pypasses after modifyingreport.py.Type of change
Spec section
None (
conformance/src/trace_tests/report.py,conformance/tests/test_report.py, andtests/test_conformance_report_parity.py).Tests for a fix
Added
tests/test_conformance_report_parity.pyand corresponding tests inconformance/tests/test_report.py. Runningpython tools/review_bar.py --base origin/main --head HEADagainst the merge base (63e38c6) reports 6 failing on behaviour and 0 on missing names (plus 6 verifying thatrecord_digest,build,to_json, andto_htmlagree on out-of-profile diagnostic records):tests.test_conformance_report_parity::test_report_level_modules_include_tr_apr_at_level_zero(fails onmainwithAssertionError: assert ('TR-ENV', 'TR-SIG', 'TR-POL') == ('TR-ENV', 'TR-SIG', 'TR-POL', 'TR-APR'))tests.test_conformance_report_parity::test_report_build_guards_non_dict_trace[str|int|none|list|bool](5 cases; all fail onmainbecausereport.buildraisesAttributeErroron non-dictrecord["trace"])tests.test_conformance_report_parity::test_report_record_digest_remains_diagnostic[nan|inf|neg-inf|float|above-safe-int|below-safe-int](6 cases; pins single-regime agreement acrossrecord_digest,report.build,to_json, andto_htmlfor out-of-profile values)Validation commands and results:
pytest tests/test_conformance_report_parity.py -v-> passed (12 passed)pytest conformance/tests/test_report.py -v-> passed (31 passed)pytest conformance/tests -q-> passed (791 passed, 5 xpassed)python tools/review_bar.py --base origin/main --head HEAD-> passed (6 fail on behaviour, 0 fail on a missing name or file, 6 pass)python tools/check_dashes.py-> passed (no dashes outside examples/, spec/trace-v0.1.md)ruff check conformance/src/trace_tests/report.py conformance/tests/test_report.py tests/test_conformance_report_parity.py-> passed (All checks passed!)mypy --strict conformance/src/trace_tests/report.py tests/test_conformance_report_parity.py-> passed (Success: no issues found in 2 source files)Checklist
git commit -s)CHANGELOG.mdupdated (for any normative change)<!-- CHANGED: #NNN: description -->in spec text